Move derivative agg to module #91014
Conversation
This cleans up the derivative pipeline agg in a few days: 1. Moves it to the `aggregations` module ala elastic#90283 2. Drops the ceremonial interface from the result ala elastic#82273 3. Adds comprehensive REST layer tests for it ala elastic#26220 4. Removes some `ESIntegTestCase tests` that duplicated those in the `AggregatorTestCase`.
elasticsearchmachine
added
the
Team:Analytics
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
| @@ -65,12 +65,11 @@ protected SearchPlugin registerPlugin() { | |||
| return new AggregationsPlugin(); | |||
| } | |||
|
|
|||
| // TODO: the base test class should be able to get this from the search plugin? (^) | |||
There was a problem hiding this comment.
Here we're registering the high level REST client's version which isn't in the SearchPlugin.
There was a problem hiding this comment.
I wonder if we should be removing these as we go. We don't need to parse the thing any more.
There was a problem hiding this comment.
I thought we also had these tests to test the xcontent serialization (similarly how we test the binary serialization)? We lose that if we drop these tests?
There was a problem hiding this comment.
(the downside is that we need to maintain the xcontent parsing in tests...)
There was a problem hiding this comment.
Yeah. That's why I didn't nuke it. It's not obviously right. Before we had the high level REST client we never tested the round trip here - we'd just render xcontent and test in REST tests or specific xcontent renderings. Now we have randomized parsing. But it's quite heavy.
|
run elasticsearch-ci/packaging-tests-windows-sample |
1 similar comment
|
run elasticsearch-ci/packaging-tests-windows-sample |
This cleans up the derivative pipeline agg in a few days:
aggregationsmodule ala Centralize aggregations into a module #90283ESIntegTestCase teststhat duplicated those in theAggregatorTestCase.